-
-
Notifications
You must be signed in to change notification settings - Fork 226
[17.0][MIG] account_invoice_overdue_reminder: backport migration to 17.0 #523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 17.0
Are you sure you want to change the base?
[17.0][MIG] account_invoice_overdue_reminder: backport migration to 17.0 #523
Conversation
…instead of supplier invoice form
…update your mail templates) Add ability to add contacts as Cc of the reminder email (added to the Cc of the mail template) Add partner_policy with 3 options to give some choice about which contact should be selected to send reminders Access reminders from partner via Action menu
Advantages: 1) easy access to the history in the chatter of the partner. 2) no problem to access the email by other users (avoid "The requested operation cannot be completed due to security restrictions. Please contact your system administrator.")
Co-authored-by: ypapouin <y.papouin@dec-industrie.com>
Co-authored-by: ypapouin <y.papouin@dec-industrie.com>
[UPD] Update account_invoice_overdue_reminder.pot [UPD] README.rst [ADD] icon.png
…e customer invoices Move "Send overdue reminder" button to the alter banner. [UPD] Update account_invoice_overdue_reminder.pot account_invoice_overdue_reminder 14.0.1.1.0 Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: credit-control-14.0/credit-control-14.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-14-0/credit-control-14-0-account_invoice_overdue_reminder/
[UPD] Update account_invoice_overdue_reminder.pot Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: credit-control-14.0/credit-control-14.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-14-0/credit-control-14-0-account_invoice_overdue_reminder/
Translated using Weblate (French) Currently translated at 98.9% (185 of 187 strings) Translation: credit-control-14.0/credit-control-14.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-14-0/credit-control-14-0-account_invoice_overdue_reminder/fr/
account_invoice_overdue_reminder 14.0.1.1.1
…mail vals generation
…achment_ids [UPD] Update account_invoice_overdue_reminder.pot [UPD] README.rst Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: credit-control-15.0/credit-control-15.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-15-0/credit-control-15-0-account_invoice_overdue_reminder/
result_notes fields converted from fields.Text to fields.Html In mass view of steps, show buttons in tree view Fix carriage return in subject of email, which may block the email Fix crash when validating several steps at the same time in the mass update interface Forword port PR OCA#236 and OCA#240 [UPD] Update account_invoice_overdue_reminder.pot [UPD] README.rst Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: credit-control-16.0/credit-control-16.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-16-0/credit-control-16-0-account_invoice_overdue_reminder/
Remove the end step in the wizard (replaced by a notification) Remove the up_to_date boolean on the start wizard Code cleanup [UPD] Update account_invoice_overdue_reminder.pot account_invoice_overdue_reminder 16.0.1.1.0 Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: credit-control-16.0/credit-control-16.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-16-0/credit-control-16-0-account_invoice_overdue_reminder/ Update __manifest__.py "application": False,
Translated using Weblate (Dutch) Currently translated at 93.4% (170 of 182 strings) Translation: credit-control-16.0/credit-control-16.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-16-0/credit-control-16-0-account_invoice_overdue_reminder/nl/ [UPD] README.rst
Translated using Weblate (Spanish) Currently translated at 78.5% (143 of 182 strings) Translation: credit-control-16.0/credit-control-16.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-16-0/credit-control-16-0-account_invoice_overdue_reminder/es/ Translated using Weblate (Spanish) Currently translated at 100.0% (182 of 182 strings) Translation: credit-control-16.0/credit-control-16.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-16-0/credit-control-16-0-account_invoice_overdue_reminder/es/
Update menu sequence, to have "space" between the previous menu entry which is account.menu_action_account_payments_receivable with sequence=15, to be able to insert another menu in-between
Translated using Weblate (Portuguese) Currently translated at 96.1% (175 of 182 strings) Translation: credit-control-16.0/credit-control-16.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-16-0/credit-control-16-0-account_invoice_overdue_reminder/pt/ Translated using Weblate (Portuguese) Currently translated at 100.0% (182 of 182 strings) Translation: credit-control-16.0/credit-control-16.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-16-0/credit-control-16-0-account_invoice_overdue_reminder/pt/ Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: credit-control-16.0/credit-control-16.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-16-0/credit-control-16-0-account_invoice_overdue_reminder/ Translated using Weblate (Dutch) Currently translated at 93.4% (170 of 182 strings) Translation: credit-control-16.0/credit-control-16.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-16-0/credit-control-16-0-account_invoice_overdue_reminder/nl/
issues due to m2o/m2m type mismatch [BOT] post-merge updates
… the invoice Add nocreate="1" in list view of account.invoice.overdue.reminder Use the new Command syntax [UPD] Update account_invoice_overdue_reminder.pot [BOT] post-merge updates Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: credit-control-18.0/credit-control-18.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-18-0/credit-control-18-0-account_invoice_overdue_reminder/ Translated using Weblate (Italian) Currently translated at 100.0% (185 of 185 strings) Translation: credit-control-18.0/credit-control-18.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-18-0/credit-control-18-0-account_invoice_overdue_reminder/it/ Translated using Weblate (French) Currently translated at 100.0% (185 of 185 strings) Translation: credit-control-18.0/credit-control-18.0-account_invoice_overdue_reminder Translate-URL: https://translation.odoo-community.org/projects/credit-control-18-0/credit-control-18-0-account_invoice_overdue_reminder/fr/ [OU-ADD] account_invoice_overdue_reminder: Migration scripts
|
@jelenapoblet |
|
Hi, I did a full functional review and it works good, as soon as test pass I will approve it. Thanks you! |
|
@jelenapoblet |
|
@NICO-SOLUTIONS, It is recommended that the add-on has its own tests that cover its logic. For example, in this case there should be tests that cover the wizard’s logic. |
@adasatorres Also, the version for 18 doesn’t have Codecov tests either. Nils |
|
Thanks @adasatorres , fully agree, the addon should include tests that cover the wizard’s logic. @NICO-SOLUTIONS Regarding the suggestion that we add the tests ourselves: this would be more complex, since it would require us to open a PR against your repository and effectively maintain tests for code that is part of your deliverable. As mentioned, we consider the tests an integral part of your work on this module. |
|
@jelenapoblet Opening a PR against this one isn’t too complex, though. Anyway, the conclusion for now is that there won’t be any additional contributions regarding the tests. |
ff4e9d5 to
153d4c1
Compare
jelenapoblet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to add tests. I’ve completed functional testing and a general review of the changes.
rrebollo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review: Great work! The code looks good to me (LGTM). Thank you for your contribution! I've provided a few suggestions for your consideration—feel free to address them as you see fit.
Given that this PR is a backport, the comments here are not mandatory for merging. However, I believe when migrating to v19, your test suite—a great contribution!—should be migrated as well, or perhaps even incorporated into v18 as an enhancement, along with considering my code suggestions.
| # Because of the "counter" field: a single reminder action for a customer, | ||
| # the "counter" may not be the same for each invoice | ||
| invoice_id = fields.Many2one( | ||
| "account.move", string="Invoice", ondelete="cascade", readonly=True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "account.move", string="Invoice", ondelete="cascade", readonly=True | |
| "account.move", string="Invoice", ondelete="cascade" |
Consider OCA's Guidelines for migration to v17 comments on this topic:
If a field is marked as readonly=True on the model, it won't be possible to import it through the Odoo import tool, so avoid it as possible and define the readonly attribute on the views instead.
| action_result_notes = fields.Html(related="action_id.result_notes", readonly=False) | ||
| action_mail_id = fields.Many2one(related="action_id.mail_id") | ||
| action_mail_cc = fields.Char( | ||
| related="action_id.mail_id.email_cc", readonly=True, string="Cc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| related="action_id.mail_id.email_cc", readonly=True, string="Cc" | |
| related="action_id.mail_id.email_cc", string="Cc" |
This is default behavior
| action_mail_state = fields.Selection( | ||
| related="action_id.mail_id.state", string="E-mail Status" | ||
| ) | ||
| counter = fields.Integer(readonly=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| counter = fields.Integer(readonly=True) | |
| counter = fields.Integer() |
See above comment on this topic
| if action.invoice_id and action.invoice_id.move_type not in [ | ||
| "out_invoice", | ||
| "out_refund", | ||
| ]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if action.invoice_id and action.invoice_id.move_type not in [ | |
| "out_invoice", | |
| "out_refund", | |
| ]: | |
| if action.invoice_id and action.invoice_id.is_sale_document(): |
|
|
||
| commercial_partner_id = fields.Many2one( | ||
| "res.partner", | ||
| readonly=True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| readonly=True, |
Set readonly attribute on the views
| return action | ||
|
|
||
| def skip(self): | ||
| self.write({"state": "skipped"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self.write({"state": "skipped"}) | |
| self.update({"state": "skipped"}) |
| orao.create(vals) | ||
| if rec.create_activity: | ||
| mao.create(self._prepare_mail_activity()) | ||
| self.write({"state": "done"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self.write({"state": "done"}) | |
| self.state = "done" |
| mail = self.env["mail.mail"].sudo().create(mvals) | ||
| if self.company_id.overdue_reminder_attach_invoice: | ||
| attachment_ids = self._get_attachment_ids(mail) | ||
| mail.write({"attachment_ids": [Command.set(attachment_ids)]}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| mail.write({"attachment_ids": [Command.set(attachment_ids)]}) | |
| mail.attachment_ids = [Command.set(attachment_ids)] |
|
|
||
| def print_letter(self): | ||
| self.check_warnings() | ||
| self.write({"letter_printed": True}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self.write({"letter_printed": True}) | |
| self.letter_printed = True |
| elif self.update_action == "reminder_type": | ||
| if not self.reminder_type: | ||
| raise UserError(_("You must select the new reminder type.")) | ||
| actions.write({"reminder_type": self.reminder_type}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| actions.write({"reminder_type": self.reminder_type}) | |
| actions.reminder_type = self.reminder_type |
rrebollo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review: Great work! The code looks good to me (LGTM). Thank you for your contribution! I've provided a few suggestions for your consideration—feel free to address them as you see fit.
Given that this PR is a backport, the comments here are not mandatory for merging. However, I believe when migrating to v19, your test suite—a great contribution!—should be migrated as well, or perhaps even incorporated into v18 as an enhancement, along with considering my code suggestions.
|
This PR has the |
|
@jelenapoblet @rrebollo For now, I’m fine with proceeding as is, and we can revisit the suggestions together with the author—either to improve v17 or during the migration to v19 (or potentially as an enhancement for v18). From my perspective, it doesn’t make much sense to merge and forward-/backport changes partially; it would be better to apply them in one clean, consolidated step. |
|
@rrebollo |
rrebollo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review. LGTM!
account_invoice_overdue_reminder/tests/test_overdue_reminder_step.py
Outdated
Show resolved
Hide resolved
|
@NICO-SOLUTIONS I suggest to leave the tests addition in a single commit, but not to merge it with migration commits, that way would make easier for someone to cherry-pick it later. |
0652b0c to
a005f30
Compare
a005f30 to
41e096d
Compare

backport migration to 17.0